Conversation
lifeart
left a comment
lifeart
left a comment
There was a problem hiding this comment.
A few additional observations beyond the earlier thread. Nothing blocking; mainly correctness/UX polish.
One meta point: this extension declares "browser": "./dist/web/client/browserClientMain" in package.json, so web-worker compatibility (raised earlier on createRequire) is a live concern, not hypothetical.
Also — no tests were added. The tiered detection has several non-obvious branches (resolver path, walk cap, monorepo-root detection, the step-counter logic below); a small table-driven test over a tmp fixture tree would make this much safer to evolve.
evoactivity
force-pushed
the
fix/pnpm-monorepo-detection
branch
from
ccb1bd2 to
d5867d7
Compare
|
I've addressed the comments. Regarding tests, there are currently no tests setup, I have something that does run tests but I'm not particularly happy with it. I want to add a full test suite, not just for this bit of work. If we can get this merged and put out I'll work on modernising everything here and writing a test suite. |
2 participants
This adds more aggressive ember project detection methods.
I often open a pnpm monorepo package as my workspace which means
workspace.findFileswill not match files because the node_modules symlinks point outside the workspace root.This uses 5 methods to detect an ember project starting with the cheapest
Tested on my project and it works.